Support `#[macro_reexport]`ing custom derives #37542

jseyfried · 2016-11-02T22:14:27Z

This is gated behind #![feature(macro_reexport)] and #![feature(proc_macro)].
r? @nrc

jseyfried · 2016-11-02T22:20:18Z

alexcrichton · 2016-11-03T16:05:40Z

Nice!

nrc

I'm not clear on what is going on here - why do we want macro_reexport and custom derive to work together? I would like macro_reexport to disappear entirely and I don't know of any motivation for it to work with custom derive. I assume this is a step towards something else, but I'm not sure what

nrc · 2016-11-07T02:15:12Z

src/test/compile-fail-fulldeps/proc-macro/cannot-link.rs

-extern crate derive_a;
-//~^ ERROR: crates of the `proc-macro` crate type cannot be linked at runtime
-
-fn main() {}


This test should still be valid, no?

Is there a good reason to disallow it? For example, we will want to support:

extern crate derive_a; use derive_a::A; #[derive(A)] struct Foo;

I would like to maintain a strict phase distinction between macro and non-macro crates (we intend to relax this eventually, but I don't want it to creep in). I don't want the macro_use to be optional, and until we actually support useing derives, it seems that we should require it.

There is still a phase distinction -- only the macros are available from extern crate derive_a and derive_a is not linked.

until we actually support useing derives

I was going to support this behind a feature gate in a PR tomorrow...

I don't want the macro_use to be optional

Why not? Without #[macro_use] or the ability to use derives, extern crate derive_a; just defines an empty module derive_a. Maybe a warning would be appropriate, but I don't see a reason to disallow it.

OK, if useing derives will work soon, then lets allow it. As long as the non-macro_use case has sensible semantics, then I see no reason to block it.

nrc · 2016-11-07T02:15:57Z

src/test/compile-fail-fulldeps/proc-macro/shadow.rs

@@ -15,6 +15,6 @@
 #[macro_use]
 extern crate derive_a;
 #[macro_use]
-extern crate derive_a; //~ ERROR `derive_a` has already been defined
+extern crate derive_a; //~ ERROR `derive_a` has already been imported

 fn main() {}


Should we have a test that uses macro_reexport with a custom derive?

Yeah, I added a test.

nrc · 2016-11-07T02:17:49Z

src/librustdoc/visit_ast.rs

-                    let def = self.cx.sess().cstore.load_macro(def_id, self.cx.sess());
+                    let def = match self.cx.sess().cstore.load_macro(def_id, self.cx.sess()) {
+                        LoadedMacro::MacroRules(macro_rules) => macro_rules,
+                        LoadedMacro::ProcMacro(..) => continue, // FIXME


This FIXME needs more explanation

jseyfried · 2016-11-07T02:49:38Z

@nrc The main purpose of this PR is to change the metadata to support reexporting custom derives.
Once we support this, I see no reason to specifically disallow reexporting custom derives (it would actually be a couple of extra lines).

#[macro_reexport] is feature gated and I think we should deprecate it once we can pub use macros behind a feature gate (writing that PR now). Until #[macro_reexport] is removed, though, I think it should behave as consistently as possible on different kinds of macros.

nrc · 2016-11-07T03:35:47Z

Until #[macro_reexport] is removed, though, I think it should behave as consistently as possible on different kinds of macros.

OK, that makes sense.

bors · 2016-11-07T07:11:40Z

☔ The latest upstream changes (presumably #37506) made this pull request unmergeable. Please resolve the merge conflicts.

nrc · 2016-11-08T01:28:00Z

r = me, but a test seems to be broken

jseyfried · 2016-11-08T06:56:52Z

@bors r=nrc

bors · 2016-11-08T06:56:53Z

📌 Commit 8d44bd5 has been approved by nrc

bors · 2016-11-08T17:57:02Z

⌛ Testing commit 8d44bd5 with merge 9723bd2...

bors · 2016-11-08T18:58:43Z

💔 Test failed - auto-win-gnu-32-opt-rustbuild

jseyfried · 2016-11-08T23:45:57Z

@bors r=nrc

bors · 2016-11-08T23:45:58Z

📌 Commit 5173173 has been approved by nrc

Rollup of 15 pull requests - Successful merges: #36868, #37134, #37229, #37250, #37370, #37428, #37432, #37472, #37524, #37614, #37622, #37627, #37636, #37644, #37654 - Failed merges: #37463, #37542, #37645

bors · 2016-11-09T23:13:57Z

🔒 Merge conflict

bors · 2016-11-09T23:19:17Z

☔ The latest upstream changes (presumably #37670) made this pull request unmergeable. Please resolve the merge conflicts.

that is referenced by multiple `extern crate` items.

jseyfried · 2016-11-10T12:35:02Z

@bors r=nrc

bors · 2016-11-10T12:35:05Z

📌 Commit f35eff2 has been approved by nrc

bors · 2016-11-10T13:10:04Z

⌛ Testing commit f35eff2 with merge 187d989...

@nrc

Support `#[macro_reexport]`ing custom derives This is gated behind `#![feature(macro_reexport)]` and `#![feature(proc_macro)]`. r? @nrc

bors · 2016-11-10T16:18:19Z

rust-highfive assigned nrc Nov 2, 2016

jseyfried force-pushed the custom_derive_reexports branch from bf9164a to 596a773 Compare November 2, 2016 22:18

jseyfried force-pushed the custom_derive_reexports branch 2 times, most recently from ae64f33 to 39b65bd Compare November 3, 2016 04:54

jseyfried force-pushed the custom_derive_reexports branch from 39b65bd to 7012100 Compare November 5, 2016 21:08

nrc suggested changes Nov 7, 2016

View reviewed changes

jseyfried force-pushed the custom_derive_reexports branch 3 times, most recently from e4d0070 to 652ec19 Compare November 8, 2016 01:09

nrc approved these changes Nov 8, 2016

View reviewed changes

jseyfried force-pushed the custom_derive_reexports branch from 652ec19 to 8d44bd5 Compare November 8, 2016 06:15

jseyfried force-pushed the custom_derive_reexports branch from 8d44bd5 to 5173173 Compare November 8, 2016 22:30

eddyb mentioned this pull request Nov 9, 2016

Rollup of 15 pull requests #37670

Merged

nit: clean up some redundant code.

8021ede

jseyfried added 9 commits November 10, 2016 06:04

Treat extern crates more like imports (pure refactoring).

b0e13dc

Avoid building multiple reduced graphs for a crate

624a9b7

that is referenced by multiple `extern crate` items.

Clean up CrateSource.

c102d7f

Refactor explicitly_linked: bool -> dep_kind: DepKind.

a1d45d9

Register and stability check #[no_link] crates.

dd0781e

Add variants Def::Macro and Namespace::MacroNS.

85f74c0

Improve macro reexports.

872943c

Fix fallout in librustdoc.

a0a9f8c

Merge branch 'refactor_macro_reexports' into custom_derive_reexports

ad53452

jseyfried force-pushed the custom_derive_reexports branch 2 times, most recently from c5d28e8 to 76d15f4 Compare November 10, 2016 11:18

jseyfried added 2 commits November 10, 2016 11:19

Support #[macro_reexport]ing custom derives.

0a998b8

Test #[macro_reexport]ing custom derives.

f35eff2

jseyfried force-pushed the custom_derive_reexports branch from 76d15f4 to f35eff2 Compare November 10, 2016 11:19

bors added a commit that referenced this pull request Nov 10, 2016

Auto merge of #37542 - jseyfried:custom_derive_reexports, r=nrc

187d989

Support `#[macro_reexport]`ing custom derives This is gated behind `#![feature(macro_reexport)]` and `#![feature(proc_macro)]`. r? @nrc

bors merged commit f35eff2 into rust-lang:master Nov 10, 2016

bors mentioned this pull request Nov 10, 2016

rustc: Implement #[link(cfg(..))] and crt-static #37545

Merged

jseyfried deleted the custom_derive_reexports branch November 15, 2016 23:03

This was referenced Nov 15, 2016

ICE using new attributes(..) section of #[proc_macro_derive] #37788

Closed

Can't compile crate that depends on a crate that depends on a proc_macro crate #37839

Closed

jseyfried mentioned this pull request Feb 7, 2017

Tracking issue for "macro naming and modularisation" (RFC #1561) #35896

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support `#[macro_reexport]`ing custom derives #37542

Support `#[macro_reexport]`ing custom derives #37542

jseyfried commented Nov 2, 2016 •

edited

Loading

jseyfried commented Nov 2, 2016 •

edited

Loading

alexcrichton commented Nov 3, 2016

nrc left a comment

nrc Nov 7, 2016

jseyfried Nov 7, 2016

nrc Nov 7, 2016

jseyfried Nov 7, 2016

nrc Nov 7, 2016

nrc Nov 7, 2016

jseyfried Nov 7, 2016

nrc Nov 7, 2016

jseyfried Nov 7, 2016

jseyfried commented Nov 7, 2016 •

edited

Loading

nrc commented Nov 7, 2016 •

edited by jseyfried

Loading

bors commented Nov 7, 2016

nrc commented Nov 8, 2016

jseyfried commented Nov 8, 2016

bors commented Nov 8, 2016

bors commented Nov 8, 2016

bors commented Nov 8, 2016

jseyfried commented Nov 8, 2016

bors commented Nov 8, 2016

bors commented Nov 9, 2016

bors commented Nov 9, 2016

jseyfried commented Nov 10, 2016

bors commented Nov 10, 2016

bors commented Nov 10, 2016

bors commented Nov 10, 2016

Support #[macro_reexport]ing custom derives #37542

Support #[macro_reexport]ing custom derives #37542

Conversation

jseyfried commented Nov 2, 2016 • edited Loading

jseyfried commented Nov 2, 2016 • edited Loading

alexcrichton commented Nov 3, 2016

nrc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jseyfried commented Nov 7, 2016 • edited Loading

nrc commented Nov 7, 2016 • edited by jseyfried Loading

bors commented Nov 7, 2016

nrc commented Nov 8, 2016

jseyfried commented Nov 8, 2016

bors commented Nov 8, 2016

bors commented Nov 8, 2016

bors commented Nov 8, 2016

jseyfried commented Nov 8, 2016

bors commented Nov 8, 2016

bors commented Nov 9, 2016

bors commented Nov 9, 2016

jseyfried commented Nov 10, 2016

bors commented Nov 10, 2016

bors commented Nov 10, 2016

bors commented Nov 10, 2016

Support `#[macro_reexport]`ing custom derives #37542

Support `#[macro_reexport]`ing custom derives #37542

jseyfried commented Nov 2, 2016 •

edited

Loading

jseyfried commented Nov 2, 2016 •

edited

Loading

jseyfried commented Nov 7, 2016 •

edited

Loading

nrc commented Nov 7, 2016 •

edited by jseyfried

Loading